-
-
Notifications
You must be signed in to change notification settings - Fork 530
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix broken layout for header subviews in iOS #902
Conversation
) This addresses bugs like software-mansion#528 where header content gets incorrectly positioned on initial load or after rotation. I was experiencing these issues in my current app, and when I looked at the XCode view debugger I could see that RNSScreenStackHeaderConfig.subviews were having their .frame.origin.x property set to a non-zero value that moved them out of place. I'm fairly new to React Native and very new to this codebase, so I haven't totally got to the bottom of what's happening here. My guess/theory is that RN layout code is setting the position of these views as if they're part of of the regular view hierarchy, even though they're meant to be managed by a UINavigationBar. Rather than trying to fix this at a deeper level and risk introducing side effects, I've added this small workaround.
Thanks for the PR! Yeah looks like the layouts passed by yoga layout are somehow trying to apply their calculations instead of leaving the origin to 0. I will test it and let other users do it and if it works, I think we can merge it 🎉 |
Maybe instead of changing it in - (void)reactSetFrame:(CGRect)frame
{
[self setFrame:CGRectMake(0, 0, frame.size.width, frame.size.height)];
} Could you test it and if it works the same, maybe add it like this instead? |
Per @WoLewicki's suggestion in software-mansion#902, move this code down to a lower level where it makes more sense.
Brilliant! That's definitely a better place for this change. I updated the PR as you suggested and the fix still works as it should, at least in my testing. |
Co-authored-by: Wojciech Lewicki <[email protected]>
This PR overrides `reactSetFrame` inside `RNSScreenStackHeaderSubview` which manually set the `frame.origin.x` and `.y` of each RNSScreenStackHeaderConfig to zero because they're meant to be managed by a `UINavigationBar`, and instead the RN layout gives them wrong origin e.g. on rotation.
@WoLewicki this is still happening randomly when I use headerRight. Sometimes, my buttons are totally off screen. |
@hirbod I haven't spotted this issue since this change iirc. Can you provide a repro where it can be spotted? |
It is nearly impossible to reproduce. 50 calls work, 1 does not. @WoLewicki |
Seems really weird since due to this PR it should be controlled by the native side and work just fine. I cannot say much without a repro unfortunately :/ |
Just had 4 reports today, that it happened to them (currently in a private beta and rolled out our soon to be released app to around 30 people) The ONLY thing I am doing is: useEffect(() => {
navigation.setOptions({
headerRight: () => (
<Text sx={{ color: '$accent', opacity: 0.3 }} variants={['semibold']}>
{t('general.save')}
</Text>
),
});
}, []); That's it. I just navigated to that screen 30-40 times, it happen once and all the other times its just fine. |
Does this code gets triggered when you navigate to a screen? Maybe there is some race condition between setting |
Yes it gets triggered when I navigate to the screen. I did not encounter this when set directly in screens options, but I need dynamic function since I pass an onPress function there (toggling active state of my save button). |
Did you try using |
No I did not try |
Description
This addresses bugs such as #528 where header content is incorrectly positioned on initial load or after rotation. I was experiencing these issues in my current app, and when I looked at the XCode view debugger I could see that
RNSScreenStackHeaderConfig
subviews had their.frame.origin.x
property set to a non-zero value that moved them out of place.I'm fairly new to React Native and very new to this codebase, so I haven't totally got to the bottom of what's happening here. My guess/theory is that RN layout code is setting the position of these views as if they're part of of the regular view hierarchy, even though they're meant to be managed by a
UINavigationBar
. Rather than trying to fix this at a deeper level and risk introducing side effects, I've added a small workaround.Fixes #528.
Changes
This PR adds a few lines of code inside
RNSScreen.viewDidLayoutSubviews
which manually set the.frame.origin.x
and.y
of eachRNSScreenStackHeaderConfig
to zero. I believe it's accomplishing the same thing as PR #668 (which was my starting point for this work) but in a simpler and more direct way.